test(components): add tests for Switch, Radio, Checkbox - #223

@claude review

Claude finished @Chibuzor-Nwemambu's task in 1m 51s —— View job

PR Review — Switch/Radio/Checkbox tests

• Gather context (diff, test files, config)
• Review test files (Switch, Radio, Checkbox)
• Review shared infra & config changes
• Post review feedback

Reviewed the full diff against origin/main and the components under test. This is a well-crafted, high-quality batch — the tests assert behavior (a11y roles/state, callback contracts, disabled gating) rather than snapshots, the shared infra is thoughtfully written, and the accessibilityState investigation described in the PR body is sound. No blocking issues.

What's good ✅

• Behavior-focused, not brittle. Tests query by accessibilityRole and assert accessibilityState, the onPress/onChange contract (toHaveBeenCalledWith(!checked)), and disabled gating — the things that actually matter for these controls.
• The jest.setup.ts icon mock is genuinely nice. Using Object.assign to copy static props (.font, etc.) off the real component onto the mock host is the right call — a naive () => <Text/> mock would silently break useEDS's ...MaterialCommunityIcons.font spread. The inline require + eslint-disable with an explanatory comment is correct given jest factory hoisting.
• toBeDisabled() over asserting accessibilityState.disabled for the no-onPress case. This is the right choice and dovetails with the reverted-fix investigation: Checkbox/Radio author accessibilityState={{ ..., disabled }} (the prop), but the Pressable's own disabled={!onPress || disabled} overrides accessibilityState.disabled at render time. Testing the effective rendered value via toBeDisabled() avoids coupling to that quirk. Good instinct, and the revert (c04a643→3798816) was justified.
• Config changes are tidy and self-documenting — the jest.config.cjs checklist narrowing plus the new comment block explaining why Dialog/Icon/Paper/etc. are intentionally off the list is a helpful addition. The test-utils moduleNameMapper ↔ tsconfig paths mirroring is kept in sync with comments on both sides.

Minor observations (non-blocking, no changes required)

1. Redundant test title wording. "reflects a different checked state via accessibilityState and icon for a different checked value" says "different checked" twice. Something like "reflects checked state via accessibilityState and icon" reads cleaner. Purely cosmetic.

2. Coverage asymmetry between Switch and Radio/Checkbox (worth a note, not a fix). Radio/Checkbox get a "non-interactive … when onPress is not provided" test because they set disabled={!onPress || disabled}. Switch has no analog because onChange defaults to () => null and disabled={disabled} only — so Switch is always interactive unless disabled is explicitly set. That's a genuine component-design inconsistency across the three controls (Switch can't be made non-interactive by omitting its handler), but it's pre-existing and out of scope here. If it's not already tracked, it'd be a reasonable entry for #152.

3. UNSAFE_getByProps({ name: ... }) couples to icon glyph names (checkbox-marked, radiobox-blank, minus-box). This is acceptable — the mapping is stable and the alternative (asserting nothing about the rendered icon) is weaker. Just flagging that a future icon-name refactor would need to touch these tests. No action needed.

Verification

I did not re-run the suite locally, but the assertions line up with the component source (Checkbox.tsx:98-118, Radio.tsx:84-101, Switch.tsx:55-73), and the PR reports pnpm test (22/22), lint, check-types, and build all clean.

Verdict: LGTM 👍 — merge-ready. The two substantive notes (Switch interactivity asymmetry, icon-name coupling) are follow-up/tracking material, not changes to this PR.
• 214-batch-3-tests

Added Dialog to the Slice 2-4 exclude lists in tsconfig.json, eslint.config.js, and jest.config.cjs. It was initially miscategorized as an internal component (like ErrorBoundary/Icon/Label/Portal/Scrim) that wouldn't get a dedicated test file, but it turns out Dialog still uses the old token system and hasn't actually been migrated yet, so it belongs on the "excluded until migrated" list instead.